-
Notifications
You must be signed in to change notification settings - Fork 19
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Issue #417: Abstract the Qbeast Snapshot Module #411
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few comments. I like that this PR also solve another issue, when we were reloading the deltaLog multiple times in the IndexTables, instead of only once for each QbeastStapshot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before merging and approving the PR, I would ask to add an issue with the scope and link the specific number instead of #398 🙏
Also, some other comments to be resolved:
Description
This is a small initial part of issue #398.
The aim of this PR is to allow the
DeltaQbeastSnapshot
to accept thetableID
as an input parameter instead of a Deltasnapshot
. This will make the top-levelQbeastSnapshot
class more abstract and enable better integration with other table formats.Related tests have also been adapted to remove the strict dependency on
org.apache.spark.sql.delta.DeltaLog
. In the future, the tests will instantiate aQbeastSnapshot
instead of aDeltaQbeastSnapshot
to make them more generic.Type of change
Refactoring
Checklist:
Here is the list of things you should do before submitting this pull request: